Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add int64, uint64 data type support for reduceL1, reduceProduct, reduceSum and reduceSumSquare #750

Closed
wants to merge 1 commit into from

Conversation

shiyi9801
Copy link
Contributor

@shiyi9801 shiyi9801 commented Aug 14, 2024

Fixes #694


Preview | Diff

@shiyi9801
Copy link
Contributor Author

Current Chromium implementation allows int64/uint64 for these reduce ops, so I think we should either change the spec or the implementation. PTAL, thanks :) @huningxin @fdwr

Copy link
Collaborator

@fdwr fdwr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opSupportLimits for CoreML will be negative for these ops, but LGTM. Thanks for spotting the difference.

@huningxin
Copy link
Contributor

Thanks @shiyi9801 , is this PR duplicated by #695?

@fdwr
Copy link
Collaborator

fdwr commented Aug 15, 2024

Thanks @shiyi9801 , is this PR duplicated by #695?

Aah yes, that did look familiar.

@shiyi9801
Copy link
Contributor Author

Thanks @shiyi9801 , is this PR duplicated by #695?

Oh I didn't notice there is already a PR to fix this, yes it is duplicated, please continue yours :) I'll drop this PR.

@shiyi9801 shiyi9801 closed this Aug 15, 2024
@shiyi9801 shiyi9801 deleted the reduce branch August 15, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding int64/uint64 data type support for some reduce operators
3 participants